Skip to content

[WIP] Add SqlQueryBuilder wrapper #457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

[WIP] Add SqlQueryBuilder wrapper #457

wants to merge 5 commits into from

Conversation

piaste
Copy link
Contributor

@piaste piaste commented Sep 29, 2017

I think it's really bad UX that the library lets you use unsupported query operators without any compile-time warning, and you have to look in the documentation to find out which ones are missing. People keep being surprised by that.

Although it would obviously be ideal to implement groupJoin and the other missing ones, I figured that there could be a simpler solution first: provide a sqlQuery computation expression where the unsupported operators are simply not available.

The sql builder in this PR is a dumb calque of the original query CE. The queries are still regular IQueryables so you can freely mix and match sql and query without issue (and it doesn't break existing code). As you can see, the tests that use unsupported operators become compile-time failures.

Other safety improvements that could potentially be achieved through this CE:

  • I think it should be possible, and I would really like to, replace the !! operator with a more intuitive naturalJoin operator, like this:
for cust in dc.Main.Customers do
naturalJoin order in cust.``main.Orders by CustomerID`` do
select (order, cust)

but I'm having trouble making it work - it doesn't seem like CustomOperationAttribute lets you define an operator that is "like for". Plus I'm not sure if the !! operator would even be correctly recognized by the code in SqlRuntime.Linq.fs this way

Thoughts?

@Thorium
Copy link
Member

Thorium commented Sep 29, 2017 •
edited
Loading

I know I'm minority in this case but I think LINQ-query wrapper is ok, and we should implement the whole LINQ support rather than limiting ourselves to some custom developer interface which is not familiar to anyone. Most of the developers can do either SQL or LINQ. Actually the API for developers may be better as SQL like https://github.com/rspeele/Rezoom.SQL has done it.

People can also use direct LINQ methods with SQLProvider by opening System.Linq and writhing very near the syntax of C# that they are used to.

And I think it's a good thing that we have multiple choices for a data access inside FSharp:
http://fsprojects.github.io/FSharp.Data.SqlClient/comparison.html

@Thorium
Copy link
Member

Thorium commented Sep 29, 2017

That being said if this is just a facade with good integration points, it could be nice addition. One thing to consider in general is how complex nested queries we want to support.

The current query didn't let me do countAsync etc. extensions. So they are now at Seq.headAsync etc, not an optimal place.

@piaste
Copy link
Contributor Author

piaste commented Sep 29, 2017

Yes, it is as pure a wrapper around the LINQ query builder as one can write (in fact, I need to add a MIT attribution notice since I literally started by regex-replacing over Query.fsi). All the calls have the same name, parameters, and XML comments as the LINQ ones, and are forwarded inline straight to query without any business code at all. The only difference, so far, is that unsupported methods are omitted.

In fact, if the compatibility with LINQ ever gets to 100%, it would be possible then to simply delete all of this file and replace it with let sql = QueryBuilder(). Any additional operators could become extension methods to QueryBuilder. (Speaking of which, do you remember which kind of problems you ran into with adding countAsync? This seems to work.)

And, assuming it's doable in the first place, I can see the case against adding any SQL-specific operators such as naturalJoin, in order to keep things as clear as possible. Thing is, currently people have to learn about the !! operator and how to use it to get a left-join, so I think it would be an improvement if I could get that operator to at least to pop up in Intellisense when you type join. Possible counter-argument: it's implemented in a way that's totally different from the regular LINQ operations, so it should appear different. I'm not sure.

@Thorium
Copy link
Member

Thorium commented Sep 30, 2017

fsharp/fslang-suggestions#495
Yes it would work with custom query, but not with F# default.

@Thorium Thorium marked this pull request as draft December 1, 2023 08:56
@Thorium Thorium closed this Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@piaste @Thorium